Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 4510 - new "enroll in the beta" contribute item #4938

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • New "Enroll in the beta" item in Settings / Contribute

Screenshot

Screenshot_1704115957

Fixes bug(s)

Impacted files

  • app_en.arb: added an "enroll in the beta" label.
  • app_store.dart: new getEnrollInBetaURL method
  • apple_app_store.dart: implements new getEnrollInBetaURL method
  • google_play.dart: implements new getEnrollInBetaURL method
  • user_preferences_contribute.dart: added an "enroll in the beta" button

Impacted files:
* `app_en.arb`: added an "enroll in the beta" label.
* `app_store.dart`: new `getEnrollInBetaURL` method
* `apple_app_store.dart`: implements new `getEnrollInBetaURL` method
* `google_play.dart`: implements new `getEnrollInBetaURL` method
* `user_preferences_contribute.dart`: added an "enroll in the beta" button
@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2024

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (3a0a2ad) 9.65% compared to head (e6b3c3b) 9.66%.
Report is 8 commits behind head on develop.

Files Patch % Lines
...pages/preferences/user_preferences_contribute.dart 5.00% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #4938      +/-   ##
==========================================
+ Coverage     9.65%   9.66%   +0.01%     
==========================================
  Files          323     322       -1     
  Lines        16207   16197      -10     
==========================================
+ Hits          1564    1565       +1     
+ Misses       14643   14632      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@teolemon teolemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monsieurtanuki
Copy link
Contributor Author

@teolemon Instead of a bottom dialog, what about this:
Screenshot_1704127195

Regarding the icon, it looks like it's not accessible with flutter (cf. https://fonts.google.com/icons?icon.query=labs).

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the red warning to be in a model or dialog to not draw all attention but codewise good 👍🏼

@monsieurtanuki
Copy link
Contributor Author

@M123-dev I'll have to code a bit anyway, my screenshot was just a suggestion without a pushed code.

@monsieurtanuki
Copy link
Contributor Author

Done:
Screenshot_1704702056

@monsieurtanuki
Copy link
Contributor Author

@teolemon For the record there's no space before an exclamation mark in English.
I cannot do anything about the test tube icon. Should the PR still be blocked?

@teolemon
Copy link
Member

teolemon commented Jan 8, 2024

What about using the SVG version ?

labs_FILL0_wght400_GRAD0_opsz24

experiment_FILL0_wght400_GRAD0_opsz24

@monsieurtanuki
Copy link
Contributor Author

@teolemon If it's just a matter of filling the test tube that can be done:
Capture d’écran 2024-01-08 à 16 32 44

@teolemon
Copy link
Member

teolemon commented Jan 8, 2024

That looks more in line with the other icons 👍
Ideally I would have preferred a material sanctionned icon, but filled will do.

@monsieurtanuki monsieurtanuki dismissed teolemon’s stale review January 8, 2024 15:46

All requested changes are there, now.

@monsieurtanuki monsieurtanuki merged commit 642892e into openfoodfacts:develop Jan 8, 2024
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev and @teolemon for your reviews!

Ideally I would have preferred a material sanctionned icon, but filled will do.

I didn't know that. Please create an issue if relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a shortcut in settings to enroll easily in the beta
4 participants